bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest Auth by sbalousek · Pull Request #18338 · python/cpython

sbalousek · 2020-02-03T23:50:41Z

The 'qop' value in the 'WWW-Authenticate' header is optional. The
presence of 'qop' in the header should be checked before its value
is parsed with 'split'.

Signed-off-by: Stephen Balousek stephen@balousek.net

https://bugs.python.org/issue39548

…ntication - The 'qop' value in the 'WWW-Authenticate' header is optional. The presence of 'qop' in the header should be checked before its value is parsed with 'split'. Signed-off-by: Stephen Balousek <stephen@balousek.net>

the-knights-who-say-ni · 2020-02-03T23:50:44Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@sbalousek

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

codecov · 2020-02-04T00:35:44Z

Codecov Report

Merging #18338 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #18338     +/-   ##
=========================================
  Coverage   82.11%   82.11%             
=========================================
  Files        1955     1954      -1     
  Lines      588601   583267   -5334     
  Branches    44406    44406             
=========================================
- Hits       483324   478961   -4363     
+ Misses      95628    94664    -964     
+ Partials     9649     9642      -7

Impacted Files	Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py	`30.00% <0.00%> (-65.00%)`	⬇️
Lib/distutils/command/bdist_rpm.py	`7.63% <0.00%> (-56.88%)`	⬇️
Lib/test/test_urllib2net.py	`76.92% <0.00%> (-13.85%)`	⬇️
Lib/test/test_smtpnet.py	`78.57% <0.00%> (-7.15%)`	⬇️
Lib/ftplib.py	`63.85% <0.00%> (-6.06%)`	⬇️
Lib/test/test_ftplib.py	`87.11% <0.00%> (-4.72%)`	⬇️
Tools/scripts/db2pickle.py	`17.82% <0.00%> (-3.97%)`	⬇️
Tools/scripts/pickle2db.py	`16.98% <0.00%> (-3.78%)`	⬇️
Lib/test/test_socket.py	`71.94% <0.00%> (-3.77%)`	⬇️
Lib/test/test_asyncio/test_base_events.py	`91.84% <0.00%> (-3.30%)`	⬇️
... and 328 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5807efd...d08190c. Read the comment docs.

sbalousek · 2020-02-04T22:15:11Z

Hi!
I apologize in advance for missing any procedural steps here. This is my first merge request, and although I read the developer documentation, there's a lot of it!
Please let me know what I am missing!

Steve

csabella · 2020-02-06T12:15:30Z

Hi Steve! Welcome and thank you for the contribution to CPython. You've done a great job navigating your first PR here. :-) The bedevere/news status is red because most changes require a News entry. This allows users to see what has changed from one version to the next. Code coverage is also red, but you changed the lines around, but didn't add new logic, so I'm not sure why it's unhappy. Typically, if you introduce new logic, you need to also add tests to cover it.

…ntication - Add NEWS item Signed-off-by: Stephen Balousek <stephen@balousek.net>

sbalousek · 2020-02-06T12:43:24Z

Thanks @csabella, for the warm welcome. I figured out how to add a News item, but I am also unsure why the code coverage test results changed. I can try adding new test cases for the 'WWW-Authenticate' header if you like. My guess is that it will take me a little while to figure the test subsystem out.

This change request is all about fixing the changes made for [bpo-38686](https://bugs.python.org/issue38686), but I can certainly see the value of adding tests for this part of the library.

sbalousek · 2020-02-06T13:09:27Z

Also, how does this change get backported to versions 3.8 and 3.7? Is that automatic or something I need to initiate?

brandtbucher · 2020-02-06T16:55:11Z

Thanks for your time, and welcome to CPython @sbalousek! 😎

Sorry about the late review, I've been tied up with a couple of other things. I've tagged this for backporting, as you suggested.

brandtbucher

Looks great! We don't have many tests in this area (and coverage is passing), so this is probably good as-is. Just one small change to the NEWS entry:

Co-Authored-By: Brandt Bucher <brandtbucher@gmail.com>

brandtbucher

Looks good now! CC @orsenthil.

PypeBros · 2020-02-13T08:15:45Z

good catch.

orsenthil

LGTM. Thank you! :)

miss-islington · 2020-02-29T20:32:01Z

Thanks @sbalousek for the PR, and @orsenthil for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

miss-islington · 2020-02-29T20:32:02Z

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

bedevere-bot · 2020-02-29T20:32:14Z

GH-18711 is a backport of this pull request to the 3.7 branch.

…ythonGH-18338) * bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest authentication - The 'qop' value in the 'WWW-Authenticate' header is optional. The presence of 'qop' in the header should be checked before its value is parsed with 'split'. Signed-off-by: Stephen Balousek <stephen@balousek.net> * bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest authentication - Add NEWS item Signed-off-by: Stephen Balousek <stephen@balousek.net> * Update Misc/NEWS.d/next/Library/2020-02-06-05-33-52.bpo-39548.DF4FFe.rst Co-Authored-By: Brandt Bucher <brandtbucher@gmail.com> Co-authored-by: Brandt Bucher <brandtbucher@gmail.com> (cherry picked from commit 5e260e0) Co-authored-by: Stephen Balousek <sbalousek@users.noreply.github.com>

miss-islington · 2020-02-29T20:46:33Z

Thanks @sbalousek for the PR, and @orsenthil for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

…ythonGH-18338) * bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest authentication - The 'qop' value in the 'WWW-Authenticate' header is optional. The presence of 'qop' in the header should be checked before its value is parsed with 'split'. Signed-off-by: Stephen Balousek <stephen@balousek.net> * bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest authentication - Add NEWS item Signed-off-by: Stephen Balousek <stephen@balousek.net> * Update Misc/NEWS.d/next/Library/2020-02-06-05-33-52.bpo-39548.DF4FFe.rst Co-Authored-By: Brandt Bucher <brandtbucher@gmail.com> Co-authored-by: Brandt Bucher <brandtbucher@gmail.com> (cherry picked from commit 5e260e0) Co-authored-by: Stephen Balousek <sbalousek@users.noreply.github.com>

bedevere-bot · 2020-02-29T20:46:42Z

GH-18712 is a backport of this pull request to the 3.8 branch.

…H-18338) * bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest authentication - The 'qop' value in the 'WWW-Authenticate' header is optional. The presence of 'qop' in the header should be checked before its value is parsed with 'split'. Signed-off-by: Stephen Balousek <stephen@balousek.net> * bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest authentication - Add NEWS item Signed-off-by: Stephen Balousek <stephen@balousek.net> * Update Misc/NEWS.d/next/Library/2020-02-06-05-33-52.bpo-39548.DF4FFe.rst Co-Authored-By: Brandt Bucher <brandtbucher@gmail.com> Co-authored-by: Brandt Bucher <brandtbucher@gmail.com> (cherry picked from commit 5e260e0) Co-authored-by: Stephen Balousek <sbalousek@users.noreply.github.com>

the-knights-who-say-ni added the CLA not signed label Feb 3, 2020

bedevere-bot added the awaiting review label Feb 3, 2020

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Feb 4, 2020

sbalousek requested a review from brandtbucher February 4, 2020 21:44

csabella requested a review from orsenthil February 6, 2020 12:15

bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest authe…

40547ef

…ntication - Add NEWS item Signed-off-by: Stephen Balousek <stephen@balousek.net>

brandtbucher added needs backport to 3.7 labels Feb 6, 2020

brandtbucher requested changes Feb 6, 2020

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2020-02-06-05-33-52.bpo-39548.DF4FFe.rst Outdated

bedevere-bot added awaiting core review and removed awaiting review labels Feb 6, 2020

Update Misc/NEWS.d/next/Library/2020-02-06-05-33-52.bpo-39548.DF4FFe.rst

a76f6f0

Co-Authored-By: Brandt Bucher <brandtbucher@gmail.com>

brandtbucher approved these changes Feb 6, 2020

View reviewed changes

orsenthil approved these changes Feb 29, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Feb 29, 2020

orsenthil merged commit 5e260e0 into python:master Feb 29, 2020

bedevere-bot removed the awaiting merge label Feb 29, 2020

bedevere-bot removed the needs backport to 3.7 label Feb 29, 2020

sthagen mentioned this pull request Feb 29, 2020

bpo-39548: Fix handling of 'WWW-Authenticate' header for Digest Auth … sthagen/python-cpython#285

Merged

orsenthil added needs backport to 3.8 and removed needs backport to 3.8 labels Feb 29, 2020

bedevere-bot removed the needs backport to 3.8 label Feb 29, 2020

Uh oh!

Uh oh!

Conversation

sbalousek commented Feb 3, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Feb 3, 2020

CLA Missing

Uh oh!

codecov Bot commented Feb 4, 2020

Codecov Report

Uh oh!

sbalousek commented Feb 4, 2020

Uh oh!

csabella commented Feb 6, 2020

Uh oh!

sbalousek commented Feb 6, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbalousek commented Feb 6, 2020

Uh oh!

brandtbucher commented Feb 6, 2020

Uh oh!

brandtbucher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brandtbucher left a comment

Choose a reason for hiding this comment

Uh oh!

PypeBros commented Feb 13, 2020

Uh oh!

orsenthil left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Feb 29, 2020

Uh oh!

miss-islington commented Feb 29, 2020

Uh oh!

bedevere-bot commented Feb 29, 2020

Uh oh!

miss-islington commented Feb 29, 2020

Uh oh!

bedevere-bot commented Feb 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

sbalousek commented Feb 3, 2020 •

edited by bedevere-bot

Loading

sbalousek commented Feb 6, 2020 •

edited by bedevere-bot

Loading